Skip to content

gh-107424: avoid using lambda functions in textwrap.indent() #107426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Jul 29, 2023

As mentioned by #107374 (comment), avoiding the use of a lambda function when no predicate is specified is expected to improve the performances.

The benchmarks reported on the issue corroborate this assumption. However, I would like to know if this is only because my laptop is dying (caching .append calls do not seem to help however).

I am waiting for changes to be verified before creating a NEWS entry (should I create it in conjunction to the already created one by @methane ?)

@ghost
Copy link

ghost commented Jul 29, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@methane
Copy link
Member

methane commented Jul 30, 2023

Our goal is not only maximize performance. And textwrap.indent() is not performance critical for most applications as far as I know.

In #107374, I made the code slightly simpler by removing generator.
On the other hand, this change makes maintenance cost little higher. We need to review and test the case for predicate is passed.
That's why I didn't incude this change in my PR.

(caching .append calls do not seem to help however).

Yes. It is old technique. Recent Python has little method call overhead. So append = L.append before loop has only little venefit.

@methane methane closed this Jul 30, 2023
@picnixz picnixz deleted the gh-107424-optimize-textwrap-indent branch July 30, 2023 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants